Skip to content

[17/17] Opt-in FP16 strong typing for DLA - #2969

Draft
mgohil-png wants to merge 14 commits into
dla/review-16-dla-public-integration-and-reshape-cleanupfrom
dla/review-17-opt-in-fp16-strong-typing-for-dla
Draft

mgohil-png wants to merge 14 commits into
dla/review-16-dla-public-integration-and-reshape-cleanupfrom
dla/review-17-opt-in-fp16-strong-typing-for-dla

Conversation

@mgohil-png

@mgohil-png mgohil-png commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: new feature

Add opt-in force_fp16_activations to the public DLA API and CLI, retaining FP32 public boundaries and the default FP32 pipeline. Part 17/17 depends on part 16; merge from the bottom upward. This self-contained FP16 feature remains together under the approved source-size exception.

Use ONNX type bindings to select supported FP16 operations and retain FP32 controls, generated values, structured outputs, nested captures and opaque functions where required. Preserve runtime-overridable initializer defaults. Eligible fixed FLOAT weights use the existing ModelOpt initializer converter; malformed or unavailable weights fail validation before mutation. Supported contrib quantizers and activation retain their original domains and imports. Standard quantizers upgrade only supported imports, migrating reduction axes in nested graphs when needed.

Scaled L2 normalization and terminal projection splitting retain native contracts, ownership and generated-name safety. Projection conversion requires fixed private weights and controls, validates supported layouts and groups, and splits eligible projections at the 8192-channel limit. Batch replacements preserve unrelated Constant payload objects. Legacy generated initializer declarations remain valid.

Terminal projection splitting additionally requires the Conv result shape [1, channels, 1, 1]; nontrivial batch summation stays in the original chain. Public option documentation attaches QDQ mutual exclusion to the correct argument.

FP16 L2 decomposition prevalidates collapsed axes with the shared normalization mapping before generating ReduceSum. A removed normalization axis refuses consistently with the native FP32 route, before earlier eligible normalization branches can be mutated.

Usage

from modelopt.onnx.graph_surgery import make_dla_compatible

make_dla_compatible("input.onnx", "output.onnx", force_fp16_activations=True)
python -m modelopt.onnx.graph_surgery make-dla-compatible --input input.onnx --output output.onnx --force-fp16-activations

The public conversion contract assumes finite FP32 inputs. Operations and controls requiring FP32 retain it.

Eligible scaled last-axis L2 decomposition uses axis -1 for non-five-dimensional inputs, so generated ReduceSum follows the last runtime dimension after leading-rank promotion. Five-dimensional inputs keep the shared collapse mapping and prevalidation.

When FP16 quantizers require opset 19, legacy equal Split nodes receive num_outputs, including nested graphs. Empty optional split-input placeholders are removed when adding that attribute so ONNX Runtime recognizes the absent input. Explicit sizes and existing num_outputs remain unchanged. Six legacy equal-split cases fail before the fix; twelve no-Split, explicit-size and modern-schema controls pass. All 101 FP16 tests pass with graph checking and CPU output comparisons.

The public API and CLI help now document the existing FP16 L2 arithmetic limitation. Finite FP32 inputs can overflow squared terms or their sum, and small squares can underflow; the denominator retains its existing 2**-14 floor. Users should validate on the target runtime and leave FP16 mode disabled when the intermediates need a wider range. Numerical rescaling is intentionally deferred to preserve reference graph and weight parity. Four strict-FP16 counterexamples reproduce the limitation; the normalization algorithm is unchanged.

Removed test_excluded_initializers_are_restored_when_conversion_fails because payload validation rejects its fixture before converter rollback is reached. The existing malformed-late-weight test retains the stronger whole-model unchanged-state assertion. The FP16 normalization override also forwards the registered rank context to the shared rank stage; eight existing low-rank execution cases fail without that forwarding and eight FP32 controls remain valid. All 101 FP16 tests pass. The arithmetic recipe and the documented numerical limitation are unchanged.

Internal FP16-eligible readers of a public Squeeze output now share a private FP16 cast. The public output stays FP32; native controls and existing FP16 Cast readers keep their original inputs. Two arithmetic execution cases fail without the fix; two native-control cases remain valid. All 101 FP16 tests pass. The normalization recipe and its documented limitation are unchanged. A Windows CPU sample with 512 readers and a retained 64 MiB payload uses one shared public-output cast, takes 0.013661 seconds and adds 2.1367 MiB sampled peak RSS. Payload bytes/identity, graph checking and CPU values are verified. Single sample with 2 ms polling.

Final FP16 validation now runs ONNX inference with check_type=True and strict_mode=True, then structural checking. These match the strict typed-inference settings used by full_check, without repeating inference on a copied model. Both public-API regressions fail before the fix and now reject malformed Resize scale types before creating or replacing output files; all 91 FP16 tests pass. For 512 readers and a retained 64 MiB payload, one sequential Windows CPU comparison takes 0.655895 seconds/+261.6914 MiB sampled peak RSS before and 0.670173 seconds/+261.5703 MiB after; complete graph hashes and CPU values match. Repeating full_check instead needs +325.3555 MiB in the same sample. Single samples, 2 ms polling; GPU execution was not measured. The FP16 arithmetic module is unchanged.

Public Cast-to-FLOAT outputs now share the existing Squeeze boundary handling: arithmetic readers receive a shared private FP16 view, while the public FLOAT value, explicit Cast readers and FP32 control inputs retain their requested precision. Six INT32/INT64 regressions fail strict typed checking before the fix; four control cases retain existing behavior. All 101 FP16 tests pass, including the actual save/reload API, ReferenceEvaluator rounding checks and ORT CPU execution. ORT CPU promotes FP16 arithmetic, so rounding-sensitive values are checked with ReferenceEvaluator and representable values with ORT. One Windows CPU sample rewrites 512 readers with a retained 64 MiB payload in 0.013800 seconds and +2.1953 MiB sampled peak RSS (2 ms polling); payload bytes and identity are preserved, a single private view is shared, and every CPU output is checked. GPU execution was not measured. L2 arithmetic is unchanged.

Testing

Windows CPU: 2049 passed in 360.69 seconds across the complete restacked graph-surgery and compatibility suite with Python 3.11.18, ONNX 1.23.2 and ONNX Runtime 1.22.0. The public DLA API uses direct ONNX saving; existing initializer conversion and unrelated surgeries are preserved. Commits are signed and all commit hooks pass with the equivalent Linux mypy platform setting. GPU execution, DLA compilation and the complete Linux suite were not run locally.

python -m pytest tests/unit/onnx/graph_surgery tests/unit/onnx/test_gqa_graph_surgery.py tests/unit/onnx/quantization/test_dq_transpose_surgery.py --noconftest -o addopts= -p no:cacheprovider -q --tb=short

Before your PR is "Ready for review"

  • Backward compatible: yes; FP16 conversion remains opt-in.
  • Third-party code/new runtime dependencies: none added; contribution and security guidance reviewed.
  • Necessary tests: checked execution and before/after regressions included.
  • Changelog: public feature documentation accompanies this series.
  • Claude approval: pending. This remains a draft.

Additional Information

Base branch: dla/review-16-dla-public-integration-and-reshape-cleanup.

Merge order: PR1 (merged); Part 2 (withdrawn); then Part 3/17 → Part 4/17 → Part 5/17 → Part 6/17 → Part 7/17 → Part 8/17 → Part 9/17 → Part 10/17 → Part 11/17 → Part 12/17 → Part 13/17 → Part 14/17 → Part 15/17 → Part 16/17 → Part 17/17.

@copy-pr-bot

copy-pr-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 12 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 094bf42 and ba8c85e.

Files selected for processing: 5
  • CHANGELOG.rst
  • modelopt/onnx/graph_surgery/__main__.py
  • modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py
  • modelopt/onnx/graph_surgery/make_dla_compatible.py
  • tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a4f76e3 and 29354ea.

Files selected for processing: 5
  • CHANGELOG.rst
  • modelopt/onnx/graph_surgery/__main__.py
  • modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py
  • modelopt/onnx/graph_surgery/make_dla_compatible.py
  • tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

Walkthrough

Adds an optional --force-fp16-activations flag to DLA graph surgery. When enabled, the pipeline converts eligible internal activations and weights to FP16, retains FP32 boundaries and required FP32 regions, rewrites eligible L2 normalization, and splits matching terminal projections.

Changes: FP16 DLA Activation Conversion

Layer / File(s) Summary
Opt-in pipeline and CLI wiring
modelopt/onnx/graph_surgery/__main__.py, modelopt/onnx/graph_surgery/make_dla_compatible.py, CHANGELOG.rst
Adds the default-disabled option to the CLI and transformation entry points. The enabled pipeline applies the FP16 transforms, performs shape inference, and validates the model.
Normalization rewrite and opset handling
modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py
Decomposes eligible last-axis L2 normalization and migrates supported quantizer models to standard opset 19. Tests cover normalization, axis mapping, nested graphs, and unsupported cases.
FP16 conversion and FP32 preservation
modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py
Validates FLOAT payloads, preserves required FP32 regions, converts eligible weights and activations, and inserts boundary casts. Tests cover controls, initializers, nested graphs, generated values, naming, and validation.
Terminal projection splitting
modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py
Splits matching terminal projection chains into chunks of at most 8192 output channels. Tests cover layouts, quantized weights, ownership conditions, legacy inputs, and output values.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant run_graph_surgery
  participant dla_make_dla_compatible
  participant _apply_force_fp16_activations
  participant ONNX_shape_inference
  CLI->>run_graph_surgery: pass force_fp16_activations
  run_graph_surgery->>dla_make_dla_compatible: forward force_fp16_activations
  dla_make_dla_compatible->>_apply_force_fp16_activations: apply FP16 transform
  dla_make_dla_compatible->>ONNX_shape_inference: infer shapes and validate
Loading

Suggested labels: waiting for feedback

Suggested reviewers: cjluo-nv, hthadicherla, vishalpandya1990

Merge Risk: Low · previous review 29354

The opt-in FP16 mode is a bounded change that leaves the default FP32 pipeline unchanged. One documented limitation remains: very large activations in the L2 normalization step can overflow in FP16 and silently produce zero outputs. Users should validate accuracy on the target runtime before enabling it.

Pre-merge checks: Passed 6
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: opt-in FP16 strong typing for DLA. The scope matches the CLI, API, and graph-surgery changes.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns Passed No security anti-patterns were introduced. The reviewed Python additions contain no torch.load(..., weights_only=False), numpy.load(..., allow_pickle=True), hardcoded trust_remote_code=True, external-…
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 43 functions across 4 files. (1 skipped: 1 unsupported.)

Full details: Security Anti-Patterns

Explanation

No security anti-patterns were introduced. The reviewed Python additions contain no torch.load(..., weights_only=False), numpy.load(..., allow_pickle=True), hardcoded trust_remote_code=True, external-input eval()/exec(), or # nosec comments. No pyproject.toml or requirements.txt files changed. The added onnx.load calls are test loading, not a prohibited pattern.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 51 minutes.

@mgohil-png
mgohil-png force-pushed the dla/review-17-opt-in-fp16-strong-typing-for-dla branch from 35a0889 to 7915fb8 Compare October 23, 2026 08:49
@mgohil-png
mgohil-png force-pushed the dla/review-17-opt-in-fp16-strong-typing-for-dla branch from 7915fb8 to b3fc54a Compare October 23, 2026 08:54
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://NVIDIA.github.io/Model-Optimizer/pr-preview/pr-2969/

Built to branch gh-pages at 2026-10-29 16:02 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@mgohil-png
mgohil-png force-pushed the dla/review-17-opt-in-fp16-strong-typing-for-dla branch 2 times, most recently from 46ad771 to 9964368 Compare October 23, 2026 10:32
@mgohil-png
mgohil-png force-pushed the dla/review-17-opt-in-fp16-strong-typing-for-dla branch 2 times, most recently from 3736757 to ed8f5d7 Compare October 23, 2026 11:46
@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 11 minutes.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 30 minutes.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 27 minutes.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 21 minutes.

@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
@mgohil-png

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 6 minutes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant